Skip to content

fix: implemented a stats script and updated telemetry tests - #82

Merged
shamikkarkhanis merged 3 commits into
developfrom
feature/capr-33-implement-interaction-logging
Feb 20, 2026
Merged

fix: implemented a stats script and updated telemetry tests#82
shamikkarkhanis merged 3 commits into
developfrom
feature/capr-33-implement-interaction-logging

Conversation

@GreenJonathan

@GreenJonathan GreenJonathan commented Feb 11, 2026

Copy link
Copy Markdown

Implemented Phase 2b: In Memory Analytics for our Telemetry service.

Summary by Sourcery

Add in-memory telemetry analytics and stats demonstration for the Discord bot.

New Features:

  • Introduce TelemetryMetrics and CommandLatencyStats to track in-memory interaction, completion, and latency metrics.
  • Expose a get_metrics accessor on the Telemetry cog for use by stats-related commands and tooling.
  • Add a demo_stats script to populate and pretty-print telemetry statistics from in-memory metrics.

Enhancements:

  • Wire telemetry event dispatching to update in-memory analytics for interactions and completions.
  • Document Phase 2b in the telemetry extension docstring, clarifying in-memory metrics behavior.

Build:

  • Relax Ruff per-file ignores for tests to allow numeric literals in assertions without lint errors.

Tests:

  • Extend telemetry tests to cover interaction and completion metrics recording and ensure dispatched events update metrics.

@sourcery-ai

sourcery-ai Bot commented Feb 11, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Implements Phase 2b in-memory analytics for the telemetry cog by introducing metrics dataclasses, wiring them into the existing event pipeline, adding a demo stats script, and expanding tests and lint configuration to cover the new behavior.

Sequence diagram for_telemetry_event_processing_with_in_memory_metrics

sequenceDiagram
    actor DiscordUser
    participant DiscordClient
    participant DiscordBot
    participant Telemetry
    participant TelemetryQueue
    participant TelemetryWorker

    DiscordUser->>DiscordClient: invoke_interaction
    DiscordClient->>DiscordBot: interaction_create
    DiscordBot->>Telemetry: log_interaction(interaction, interaction_type, command_name)
    Telemetry->>TelemetryQueue: put(TelemetryEvent type=interaction, data)

    loop background_consumer
        TelemetryWorker->>TelemetryQueue: get()
        TelemetryQueue-->>TelemetryWorker: TelemetryEvent
        TelemetryWorker->>Telemetry: _dispatch_event(event)
        alt interaction_event
            Telemetry->>Telemetry: _log_interaction(data)
            Telemetry->>Telemetry: _record_interaction_metrics(data)
        else completion_event
            Telemetry->>Telemetry: _log_completion(...)
            Telemetry->>Telemetry: _record_completion_metrics(data)
        end
    end

    DiscordBot->>Telemetry: get_metrics()
    Telemetry-->>DiscordBot: TelemetryMetrics_snapshot
Loading

Class diagram for in_memory_telemetry_analytics_phase_2b

classDiagram
    class TelemetryEvent {
        +str event_type
        +dict~str, Any~ data
    }

    class CommandLatencyStats {
        +int count
        +float total_ms
        +float min_ms
        +float max_ms
        +record(duration_ms float) void
        +avg_ms float
    }

    class TelemetryMetrics {
        +datetime boot_time
        +int total_interactions
        +defaultdict~str, int~ interactions_by_type
        +defaultdict~str, int~ command_invocations
        +set~int~ unique_user_ids
        +defaultdict~int, int~ guild_interactions
        +defaultdict~str, int~ completions_by_status
        +defaultdict~str, defaultdict~str, int~~ command_failures
        +defaultdict~str, int~ error_types
        +defaultdict~str, CommandLatencyStats~ command_latency
    }

    class Telemetry {
        -commands.Bot bot
        -logging.Logger log
        -dict~int, tuple~str, float~~ _pending
        -asyncio.Queue~TelemetryEvent~ _queue
        -TelemetryMetrics _metrics
        +__init__(bot commands.Bot)
        -_dispatch_event(event TelemetryEvent) void
        +log_interaction(interaction Any, interaction_type str, command_name str) None
        +log_completion(interaction_id int, status str, duration_ms float, command_name str, error_type str) None
        +log_command_failure(command_name str, status str, error_type str) None
        +get_metrics() TelemetryMetrics
        -_record_interaction_metrics(data dict~str, Any~) None
        -_record_completion_metrics(data dict~str, Any~) None
    }

    Telemetry --> TelemetryMetrics : owns
    TelemetryEvent <.. Telemetry : consumes
    TelemetryMetrics --> CommandLatencyStats : uses
    TelemetryMetrics "*" --> "1" CommandLatencyStats : latency_per_command
Loading

File-Level Changes

Change Details Files
Add in-memory telemetry metrics data structures and wire them into the Telemetry cog lifecycle.
  • Introduce CommandLatencyStats dataclass to track per-command latency stats with O(1) memory and provide an avg_ms property.
  • Introduce TelemetryMetrics dataclass holding counters and collections for interactions, commands, users, guilds, completions, failures, error types, and per-command latency.
  • Instantiate TelemetryMetrics in Telemetry.init and store it on self._metrics for the cog lifetime.
  • Update the Telemetry module docstring to describe Phase 2b in-memory analytics and clarify design decisions and future phases.
capy_discord/exts/core/telemetry.py
Update event dispatching to feed in-memory metrics via new recording helpers.
  • Extend _dispatch_event to call _record_interaction_metrics for interaction events and _record_completion_metrics for completion events, in addition to existing logging.
  • Implement _record_interaction_metrics to increment interaction counters, command invocation counts, unique user IDs, and per-guild interaction counts while ignoring None guild IDs.
  • Implement _record_completion_metrics to track completions by status, update command latency stats, record command failures for non-success statuses, and count error types when present.
  • Expose a get_metrics accessor on the Telemetry cog to return the current TelemetryMetrics instance for use by other components (e.g., /stats).
capy_discord/exts/core/telemetry.py
Add tests validating in-memory analytics behavior of the Telemetry cog.
  • Add tests that _record_interaction_metrics correctly increments counters, handles multiple events, and skips None guild IDs for DMs.
  • Add tests that _record_completion_metrics records success and failure completions, including latency aggregation, failure breakdown by status, and error type counts.
  • Add a test that dispatching an interaction TelemetryEvent via the queue updates metrics while still performing existing logging.
tests/capy_discord/exts/test_telemetry.py
Provide a demo script to generate and print sample telemetry stats from TelemetryMetrics.
  • Add scripts/demo_stats.py which constructs a TelemetryMetrics instance with simulated interaction and completion data to resemble a real bot session.
  • Implement helper functions in the script to print a readable stats report, including header/uptime, overview, top commands with latency, interaction type counts, latency details, error summaries, and failures by command.
  • Document how to run the script using uv and python -c in the module docstring.
scripts/demo_stats.py
Adjust linting configuration to accommodate the new tests.
  • Extend Ruff per-file ignores for tests/* to additionally ignore PLR2004 (magic value) in tests alongside existing annotation, docstring, and S101 ignores.
pyproject.toml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 3 issues, and left some high level feedback:

  • Consider returning a copy or read-only view from get_metrics() (or clearly documenting it) so callers of the stats API cannot accidentally mutate the internal TelemetryMetrics state.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider returning a copy or read-only view from `get_metrics()` (or clearly documenting it) so callers of the stats API cannot accidentally mutate the internal `TelemetryMetrics` state.

## Individual Comments

### Comment 1
<location> `capy_discord/exts/core/telemetry.py:336-338` </location>
<code_context>
+    # ANALYTICS
+    # ========================================================================================
+
+    def get_metrics(self) -> TelemetryMetrics:
+        """Return the current in-memory metrics snapshot."""
+        return self._metrics
+
+    def _record_interaction_metrics(self, data: dict[str, Any]) -> None:
</code_context>

<issue_to_address>
**suggestion (bug_risk):** Returning the live TelemetryMetrics instance exposes internal state to mutation by callers.

External callers (e.g. a /stats handler) can mutate this object, affecting live counters and future readings. If this should be read-only, return a shallow copy or an immutable snapshot (e.g. a DTO), or clearly document that callers are allowed to mutate it. Using the internal instance also couples callers to this implementation and may complicate future changes (like adding locking or swapping out the metrics store).

Suggested implementation:

```python
    def get_metrics(self) -> TelemetryMetrics:
        """Return an immutable snapshot of the current in-memory metrics."""
        # Return a deep copy so external callers cannot mutate internal state.
        return copy.deepcopy(self._metrics)

```

1. Add `import copy` near the top of `capy_discord/exts/core/telemetry.py` alongside the other imports, for example:

   `import copy`

2. If you prefer not to deep copy (e.g. for performance reasons), you could instead:
   - Introduce a DTO or `@dataclass` that represents a read-only snapshot and construct it from `self._metrics`, or
   - Add a `to_snapshot()`/`copy()` method on `TelemetryMetrics` and call that from `get_metrics` instead of `copy.deepcopy`.
</issue_to_address>

### Comment 2
<location> `capy_discord/exts/core/telemetry.py:361-363` </location>
<code_context>
+    def _record_completion_metrics(self, data: dict[str, Any]) -> None:
+        """Update in-memory counters from a completion event."""
+        m = self._metrics
+        status = data.get("status", "unknown")
+        command_name = data.get("command_name", "unknown")
+        duration_ms = data.get("duration_ms", 0.0)
+
+        m.completions_by_status[status] += 1
</code_context>

<issue_to_address>
**issue (bug_risk):** Defaulting missing duration_ms to 0.0 risks skewing latency metrics and hides data issues.

Using 0.0 makes missing durations indistinguishable from real near-zero latencies and will bias aggregates downward, while also masking upstream data problems. Instead, consider either skipping latency updates when duration_ms is missing/None, or logging a warning and not updating command_latency for that event so malformed events are visible and don’t affect stats.
</issue_to_address>

### Comment 3
<location> `tests/capy_discord/exts/test_telemetry.py:275-283` </location>
<code_context>
+    assert m.error_types["UserFriendlyError"] == 1
+
+
+def test_record_completion_metrics_latency_stats(cog):
+    cog._record_completion_metrics({"command_name": "ping", "status": "success", "duration_ms": 10.0})
+    cog._record_completion_metrics({"command_name": "ping", "status": "success", "duration_ms": 30.0})
+
+    stats = cog.get_metrics().command_latency["ping"]
+    assert stats.count == 2
+    assert stats.avg_ms == 20.0
+    assert stats.min_ms == 10.0
+    assert stats.max_ms == 30.0
+
+
</code_context>

<issue_to_address>
**suggestion (testing):** Add a unit test for CommandLatencyStats with zero observations

This exercises the multi-observation path, but there’s no coverage for the zero-observation case. Please add a small test that constructs `CommandLatencyStats` directly, verifies its initial `min_ms`/`max_ms` values, and asserts `avg_ms == 0.0` when `count` is still zero.

Suggested implementation:

```python
def test_record_completion_metrics_latency_stats(cog):
    cog._record_completion_metrics({"command_name": "ping", "status": "success", "duration_ms": 10.0})
    cog._record_completion_metrics({"command_name": "ping", "status": "success", "duration_ms": 30.0})

    stats = cog.get_metrics().command_latency["ping"]
    assert stats.count == 2
    assert stats.avg_ms == 20.0
    assert stats.min_ms == 10.0
    assert stats.max_ms == 30.0


def test_command_latency_stats_zero_observations():
    stats = CommandLatencyStats()

    # Initial state with no observations
    assert stats.count == 0
    assert stats.min_ms == float("inf")
    assert stats.max_ms == 0.0

    # avg_ms should be 0.0 when count is zero
    assert stats.avg_ms == 0.0

```

1. At the top of `tests/capy_discord/exts/test_telemetry.py`, add an import for `CommandLatencyStats`, e.g.:
   `from capy_discord.exts.telemetry import CommandLatencyStats`
   (adjust the import path to match where `CommandLatencyStats` is actually defined).
2. If `CommandLatencyStats` uses different initial values for `min_ms`/`max_ms` than `float("inf")` and `0.0`, update the corresponding assertions in `test_command_latency_stats_zero_observations` to match the real defaults.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread capy_discord/exts/core/telemetry.py Outdated
Comment thread capy_discord/exts/core/telemetry.py Outdated
Comment on lines +275 to +283
def test_record_completion_metrics_latency_stats(cog):
cog._record_completion_metrics({"command_name": "ping", "status": "success", "duration_ms": 10.0})
cog._record_completion_metrics({"command_name": "ping", "status": "success", "duration_ms": 30.0})

stats = cog.get_metrics().command_latency["ping"]
assert stats.count == 2
assert stats.avg_ms == 20.0
assert stats.min_ms == 10.0
assert stats.max_ms == 30.0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Add a unit test for CommandLatencyStats with zero observations

This exercises the multi-observation path, but there’s no coverage for the zero-observation case. Please add a small test that constructs CommandLatencyStats directly, verifies its initial min_ms/max_ms values, and asserts avg_ms == 0.0 when count is still zero.

Suggested implementation:

def test_record_completion_metrics_latency_stats(cog):
    cog._record_completion_metrics({"command_name": "ping", "status": "success", "duration_ms": 10.0})
    cog._record_completion_metrics({"command_name": "ping", "status": "success", "duration_ms": 30.0})

    stats = cog.get_metrics().command_latency["ping"]
    assert stats.count == 2
    assert stats.avg_ms == 20.0
    assert stats.min_ms == 10.0
    assert stats.max_ms == 30.0


def test_command_latency_stats_zero_observations():
    stats = CommandLatencyStats()

    # Initial state with no observations
    assert stats.count == 0
    assert stats.min_ms == float("inf")
    assert stats.max_ms == 0.0

    # avg_ms should be 0.0 when count is zero
    assert stats.avg_ms == 0.0
  1. At the top of tests/capy_discord/exts/test_telemetry.py, add an import for CommandLatencyStats, e.g.:
    from capy_discord.exts.telemetry import CommandLatencyStats
    (adjust the import path to match where CommandLatencyStats is actually defined).
  2. If CommandLatencyStats uses different initial values for min_ms/max_ms than float("inf") and 0.0, update the corresponding assertions in test_command_latency_stats_zero_observations to match the real defaults.

@shamikkarkhanis shamikkarkhanis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check the sourcery bug risk comments !

@shamikkarkhanis shamikkarkhanis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

======================================================================================================= warnings summary =======================================================================================================
tests/capy_discord/exts/test_telemetry.py::test_dispatch_unknown_event_type
tests/capy_discord/exts/test_telemetry.py::test_consumer_processes_events
tests/capy_discord/exts/test_telemetry.py::test_failure_internal_error_categorized
tests/capy_discord/exts/test_telemetry.py::test_completion_event_enqueued
/Users/shamik/Documents/capy/capy-discord/tests/capy_discord/exts/test_telemetry.py:22: DeprecationWarning: There is no current event loop
b.wait_until_ready = MagicMock(return_value=asyncio.Future())

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

take a look at these test warnings

@shamikkarkhanis
shamikkarkhanis merged commit 78d32f1 into develop Feb 20, 2026
4 checks passed
@shamikkarkhanis
shamikkarkhanis deleted the feature/capr-33-implement-interaction-logging branch February 20, 2026 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants